Skip to content

next_steps stops advertising work the checkout gate refuses - #111

Merged
Barneyjm merged 2 commits into
mainfrom
claude/next-steps-consistency
Aug 3, 2026
Merged

next_steps stops advertising work the checkout gate refuses#111
Barneyjm merged 2 commits into
mainfrom
claude/next-steps-consistency

Conversation

@Barneyjm

@Barneyjm Barneyjm commented Aug 2, 2026

Copy link
Copy Markdown
Owner

My inconsistency from #109/#110, caught by checking production after #110 deployed.

#110 blocked checkout of a task whose decomposition proposal is still out for review, and hid such tasks from both pool listings — but deriveNextSteps (added in #109) queries status='open' directly and was never taught the predicate. So the conjecture page kept offering firstproof-c4's "Simulate slim(Δ)" at 40¢ while the API refused it:

$ POST /checkout {"task_id":"c0fbb5df…"}
{"error":"decomposition_pending_review","message":"This task already has a
decomposition proposal awaiting peer review…"}

Now it uses the same pendingDecompositionSql — one definition, so the listing and the gate can't drift apart again.

The omission is counted, not silent. awaiting_decomposition_review reports how many open tasks were held back. A task missing from the list because its split is out for review is a fact about the target, not an absence — and the review task itself stays claimable, since reviewing the split is the next step. So a target in that position is correctly not stalled, and the flag accounts for it.

596 tests, 2 new.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6

#110 blocked checkout of a task whose decomposition proposal is still out for
review, and hid such tasks from both pool listings — but deriveNextSteps (added
in #109) queries status='open' directly and was never taught the predicate. So
the conjecture page kept offering firstproof-c4's "Simulate slim(Δ)" at 40c
after the API had started refusing it with decomposition_pending_review.

Same predicate, one definition, so the two cannot drift again.

The omission is counted, not silent: awaiting_decomposition_review reports how
many open tasks were held back, because a task missing from the list because
its split is out for review is a fact about the target rather than an absence.
The review task itself stays claimable — it is the actual next step — so a
target in this position is not stalled, and the stalled flag accounts for it.

596 tests (2 new).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Four parallel review agents (reuse / simplification / efficiency / altitude)
over the #109#111 surface. Applied:

LEAK. deriveNextSteps filtered only status='open' and was missing the two
exclusions listAvailableTasks documents — onboarding_dev_id IS NULL and
sensitivity = 'public'. getTargetProgress is unauthenticated, and the
onboarding target is public and slugged, so next_steps.claimable would
advertise per-dev onboarding tasks nobody else can claim, plus non-public
tasks on public conjectures. They stay in the status counts; they never
reach `claimable`.

THE TOMBSTONE WAS THE WRONG SHAPE. boundedStateUpdate kept the byte TAIL of
the serialized state under {truncated, note, tail} — not parseable JSON, so
unreadable to the next agent, and an object of alien keys that then forced
mergeStateUpdate to duck-type the marker so a later write wouldn't inherit
it. Bounding is now key-aware: drop whole keys largest-first, name them in
`_dropped`, and the stored value stays a real working set at every size. The
isTombstone special case goes with its cause. Byte-preserving only mattered
while established results lived in the blob; they live in target_facts now.
`_dropped` is platform-owned and cleared on every merge, so one overflow
cannot brand a working set forever.

THE FIXED PATTERN HAD AN UNFIXED TWIN. salvageCrashedRun still spread the
prior state back over itself, with a comment claiming it matched the timeout
salvage — which #110 had changed. Both now send only their own key.

EFFICIENCY. The per-fact INSERT loop ran one round trip per claim while
holding FOR UPDATE on the targets row, with nothing capping how many claims
an agent may send; now one unnest insert. getTargetProgress's three
independent reads run concurrently. checkout folds the pending-decomposition
predicate into the task SELECT it already runs, instead of a second round
trip inside the money-path transaction. Migration 016 adds the two missing
indexes: tasks(target_id, status) for the per-page-view scan, and a partial
expression index for the review_of JSONB probe.

TIDYING. deriveNextSteps counts in one pass, not five filters. The dead
synthesizeWorkUnitSummary alias is gone. ParsedStateUpdate.state is typed
`unknown`, which is what it always was.

Skipped, with reasons in the reply: hydrating facts into checkout (real gap,
own PR), facts/$retract as envelope siblings rather than reserved keys
(interface decision), a real review_of column (larger migration), one shared
isPlainObject (five files outside this diff).

595 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
@Barneyjm
Barneyjm merged commit efeb6d9 into main Aug 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant